From 493519fd231c627f3356107ba0f819870090b180 Mon Sep 17 00:00:00 2001 From: "cl349@firebug.cl.cam.ac.uk" Date: Mon, 12 Sep 2005 21:12:16 +0000 Subject: [PATCH] Change xenbus_dev interface from ioctl to read/write. Check boundaries so we can recover if userspace dies. Also simplifies libxenstore. Signed-off-by: Rusty Russell Signed-off-by: Christian Limpach --- .hgignore | 1 - .../drivers/xen/xenbus/xenbus_dev.c | 162 +++++++++--------- .../drivers/xen/xenbus/xenbus_xs.c | 8 +- .../include/asm-xen/linux-public/xenbus_dev.h | 47 ----- tools/xenstore/Makefile | 1 - tools/xenstore/xs.c | 62 +------ 6 files changed, 89 insertions(+), 192 deletions(-) delete mode 100644 linux-2.6-xen-sparse/include/asm-xen/linux-public/xenbus_dev.h diff --git a/.hgignore b/.hgignore index 07be3c5255..42e487b3b1 100644 --- a/.hgignore +++ b/.hgignore @@ -152,7 +152,6 @@ ^tools/xenstat/xentop/xentop$ ^tools/xenstore/testsuite/tmp/.*$ ^tools/xenstore/xen$ -^tools/xenstore/xenbus_dev.h$ ^tools/xenstore/xenstored$ ^tools/xenstore/xenstored_test$ ^tools/xenstore/xenstore-read$ diff --git a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_dev.c b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_dev.c index 494b0f7cc4..68e42d8672 100644 --- a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_dev.c +++ b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_dev.c @@ -5,6 +5,7 @@ * to xenstore. * * Copyright (c) 2005, Christian Limpach + * Copyright (c) 2005, Rusty Russell, IBM Corporation * * This file may be distributed separately from the Linux kernel, or * incorporated into other software packages, subject to the following license: @@ -41,105 +42,97 @@ #include #include -#include #include struct xenbus_dev_data { - int in_transaction; + /* Are there bytes left to be read in this message? */ + int bytes_left; + /* Are we still waiting for the reply to a message we wrote? */ + int awaiting_reply; + /* Buffer for outgoing messages. */ + unsigned int len; + union { + struct xsd_sockmsg msg; + char buffer[PAGE_SIZE]; + } u; }; static struct proc_dir_entry *xenbus_dev_intf; -void *xs_talkv(enum xsd_sockmsg_type type, const struct kvec *iovec, - unsigned int num_vecs, unsigned int *len); - -static int xenbus_dev_talkv(struct xenbus_dev_data *u, unsigned long data) +/* Reply can be long (dir, getperm): don't buffer, just examine + * headers so we can discard rest if they die. */ +static ssize_t xenbus_dev_read(struct file *filp, + char __user *ubuf, + size_t len, loff_t *ppos) { - struct xenbus_dev_talkv xt; - unsigned int len; - void *resp, *base; - struct kvec *iovec; - int ret = -EFAULT, v = 0; - - if (copy_from_user(&xt, (void *)data, sizeof(xt))) - return -EFAULT; - - iovec = kmalloc(xt.num_vecs * sizeof(struct kvec), GFP_KERNEL); - if (iovec == NULL) - return -ENOMEM; - - if (copy_from_user(iovec, xt.iovec, - xt.num_vecs * sizeof(struct kvec))) - goto out; - - for (v = 0; v < xt.num_vecs; v++) { - base = iovec[v].iov_base; - iovec[v].iov_base = kmalloc(iovec[v].iov_len, GFP_KERNEL); - if (iovec[v].iov_base == NULL || - copy_from_user(iovec[v].iov_base, base, iovec[v].iov_len)) - { - if (iovec[v].iov_base) - kfree(iovec[v].iov_base); - else - ret = -ENOMEM; - v--; - goto out; - } + struct xenbus_dev_data *data = filp->private_data; + struct xsd_sockmsg msg; + int err; + + /* Refill empty buffer? */ + if (data->bytes_left == 0) { + if (len < sizeof(msg)) + return -EINVAL; + + err = xb_read(&msg, sizeof(msg)); + if (err) + return err; + data->bytes_left = msg.len; + if (ubuf && copy_to_user(ubuf, &msg, sizeof(msg)) != 0) + return -EFAULT; + /* We can receive spurious XS_WATCH_EVENT messages. */ + if (msg.type != XS_WATCH_EVENT) + data->awaiting_reply = 0; + return sizeof(msg); } - resp = xs_talkv(xt.type, iovec, xt.num_vecs, &len); - if (IS_ERR(resp)) { - ret = PTR_ERR(resp); - goto out; - } + /* Don't read over next header, or over temporary buffer. */ + if (len > sizeof(data->u.buffer)) + len = sizeof(data->u.buffer); + if (len > data->bytes_left) + len = data->bytes_left; - switch (xt.type) { - case XS_TRANSACTION_START: - u->in_transaction = 1; - break; - case XS_TRANSACTION_END: - u->in_transaction = 0; - break; - default: - break; - } - - ret = len; - if (len > xt.len) - len = xt.len; + err = xb_read(data->u.buffer, len); + if (err) + return err; - if (copy_to_user(xt.buf, resp, len)) - ret = -EFAULT; - - kfree(resp); - out: - while (v-- > 0) - kfree(iovec[v].iov_base); - kfree(iovec); - return ret; + data->bytes_left -= len; + if (ubuf && copy_to_user(ubuf, data->u.buffer, len) != 0) + return -EFAULT; + return len; } -static int xenbus_dev_ioctl(struct inode *inode, struct file *filp, - unsigned int cmd, unsigned long data) +/* We do v. basic sanity checking so they don't screw up kernel later. */ +static ssize_t xenbus_dev_write(struct file *filp, + const char __user *ubuf, + size_t len, loff_t *ppos) { - struct xenbus_dev_data *u = filp->private_data; - int ret = -ENOSYS; - - switch (cmd) { - case IOCTL_XENBUS_DEV_TALKV: - ret = xenbus_dev_talkv(u, data); - break; - default: - ret = -EINVAL; - break; + struct xenbus_dev_data *data = filp->private_data; + int err; + + /* We gather data in buffer until we're ready to send it. */ + if (len > data->len + sizeof(data->u)) + return -EINVAL; + if (copy_from_user(data->u.buffer + data->len, ubuf, len) != 0) + return -EFAULT; + data->len += len; + if (data->len >= sizeof(data->u.msg) + data->u.msg.len) { + err = xb_write(data->u.buffer, data->len); + if (err) + return err; + data->len = 0; + data->awaiting_reply = 1; } - return ret; + return len; } static int xenbus_dev_open(struct inode *inode, struct file *filp) { struct xenbus_dev_data *u; + /* Don't try seeking. */ + nonseekable_open(inode, filp); + u = kmalloc(sizeof(*u), GFP_KERNEL); if (u == NULL) return -ENOMEM; @@ -155,20 +148,25 @@ static int xenbus_dev_open(struct inode *inode, struct file *filp) static int xenbus_dev_release(struct inode *inode, struct file *filp) { - struct xenbus_dev_data *u = filp->private_data; + struct xenbus_dev_data *data = filp->private_data; + + /* Discard any unread replies. */ + while (data->bytes_left || data->awaiting_reply) + xenbus_dev_read(filp, NULL, sizeof(data->u.buffer), NULL); - if (u->in_transaction) - xenbus_transaction_end(1); + /* Harmless if no transaction in progress. */ + xenbus_transaction_end(1); up(&xenbus_lock); - kfree(u); + kfree(data); return 0; } static struct file_operations xenbus_dev_file_ops = { - .ioctl = xenbus_dev_ioctl, + .read = xenbus_dev_read, + .write = xenbus_dev_write, .open = xenbus_dev_open, .release = xenbus_dev_release, }; diff --git a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_xs.c b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_xs.c index 9938f81257..73e21ae3a2 100644 --- a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_xs.c +++ b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_xs.c @@ -106,10 +106,10 @@ void xenbus_debug_write(const char *str, unsigned int count) } /* Send message to xs, get kmalloc'ed reply. ERR_PTR() on error. */ -void *xs_talkv(enum xsd_sockmsg_type type, - const struct kvec *iovec, - unsigned int num_vecs, - unsigned int *len) +static void *xs_talkv(enum xsd_sockmsg_type type, + const struct kvec *iovec, + unsigned int num_vecs, + unsigned int *len) { struct xsd_sockmsg msg; void *ret = NULL; diff --git a/linux-2.6-xen-sparse/include/asm-xen/linux-public/xenbus_dev.h b/linux-2.6-xen-sparse/include/asm-xen/linux-public/xenbus_dev.h deleted file mode 100644 index 94256ec9e6..0000000000 --- a/linux-2.6-xen-sparse/include/asm-xen/linux-public/xenbus_dev.h +++ /dev/null @@ -1,47 +0,0 @@ -/* - * xenbus_dev.h - * - * Copyright (c) 2005, Christian Limpach - * - * This file may be distributed separately from the Linux kernel, or - * incorporated into other software packages, subject to the following license: - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this source file (the "Software"), to deal in the Software without - * restriction, including without limitation the rights to use, copy, modify, - * merge, publish, distribute, sublicense, and/or sell copies of the Software, - * and to permit persons to whom the Software is furnished to do so, subject to - * the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS - * IN THE SOFTWARE. - */ - -#ifndef _XENBUS_DEV_H_ -#define _XENBUS_DEV_H_ - -struct xenbus_dev_talkv { - enum xsd_sockmsg_type type; - const struct kvec *iovec; - unsigned int num_vecs; - char *buf; - unsigned int len; -}; - -/* - * @cmd: IOCTL_XENBUS_DEV_TALKV - * @arg: struct xenbus_dev_talkv - * Return: 0 on success, error code on failure. - */ -#define IOCTL_XENBUS_DEV_TALKV \ - _IOC(_IOC_NONE, 'X', 0, sizeof(struct xenbus_dev_talkv)) - -#endif /* _XENBUS_DEV_H_ */ diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile index 94a4f41381..3ed0d7cff0 100644 --- a/tools/xenstore/Makefile +++ b/tools/xenstore/Makefile @@ -17,7 +17,6 @@ BASECFLAGS+= -O3 $(PROFILE) BASECFLAGS+= -I$(XEN_ROOT)/tools/libxc BASECFLAGS+= -I$(XEN_ROOT)/xen/include/public BASECFLAGS+= -I. -BASECFLAGS+= -I$(XEN_ROOT)/linux-2.6-xen-sparse/include/asm-xen/linux-public CFLAGS += $(BASECFLAGS) LDFLAGS += $(PROFILE) -L$(XEN_LIBXC) diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c index 5d36a8d642..95fc942c4a 100644 --- a/tools/xenstore/xs.c +++ b/tools/xenstore/xs.c @@ -36,12 +36,10 @@ #include "xenstored.h" #include "xs_lib.h" #include "utils.h" -#include "xenbus_dev.h" struct xs_handle { int fd; - enum { SOCK, DEV } type; }; /* Get the socket from the store daemon handle. @@ -68,7 +66,6 @@ static struct xs_handle *get_socket(const char *connect_to) h = malloc(sizeof(*h)); if (h) { h->fd = sock; - h->type = SOCK; return h; } } @@ -82,16 +79,15 @@ static struct xs_handle *get_socket(const char *connect_to) static struct xs_handle *get_dev(const char *connect_to) { int fd, saved_errno; - struct xs_handle *h = NULL; + struct xs_handle *h; - fd = open(connect_to, O_RDONLY); + fd = open(connect_to, O_RDWR); if (fd < 0) return NULL; h = malloc(sizeof(*h)); if (h) { h->fd = fd; - h->type = DEV; return h; } @@ -190,9 +186,9 @@ static void *read_reply(int fd, enum xsd_sockmsg_type *type, unsigned int *len) } /* Send message to xs, get malloc'ed reply. NULL and set errno on error. */ -static void *xs_talkv_sock(struct xs_handle *h, enum xsd_sockmsg_type type, - const struct iovec *iovec, unsigned int num_vecs, - unsigned int *len) +static void *xs_talkv(struct xs_handle *h, enum xsd_sockmsg_type type, + const struct iovec *iovec, unsigned int num_vecs, + unsigned int *len) { struct xsd_sockmsg msg; void *ret = NULL; @@ -253,54 +249,6 @@ close_fd: return NULL; } -/* Send message to xs, get malloc'ed reply. NULL and set errno on error. */ -static void *xs_talkv_dev(struct xs_handle *h, enum xsd_sockmsg_type type, - const struct iovec *iovec, unsigned int num_vecs, - unsigned int *len) -{ - struct xenbus_dev_talkv dt; - char *buf; - int err, buflen = 1024; - - again: - buf = malloc(buflen); - if (buf == NULL) { - errno = ENOMEM; - return NULL; - } - dt.type = type; - dt.iovec = (struct kvec *)iovec; - dt.num_vecs = num_vecs; - dt.buf = buf; - dt.len = buflen; - err = ioctl(h->fd, IOCTL_XENBUS_DEV_TALKV, &dt); - if (err < 0) { - free(buf); - errno = err; - return NULL; - } - if (err > buflen) { - free(buf); - buflen = err; - goto again; - } - if (len) - *len = err; - return buf; -} - -/* Send message to xs, get malloc'ed reply. NULL and set errno on error. */ -static void *xs_talkv(struct xs_handle *h, enum xsd_sockmsg_type type, - const struct iovec *iovec, unsigned int num_vecs, - unsigned int *len) -{ - if (h->type == SOCK) - return xs_talkv_sock(h, type, iovec, num_vecs, len); - if (h->type == DEV) - return xs_talkv_dev(h, type, iovec, num_vecs, len); - return NULL; -} - /* free(), but don't change errno. */ static void free_no_errno(void *p) { -- 2.30.2